Add Workspace Rooms Create Button Integration under workspaceRoomsPage beta#90953
Conversation
|
has been reviewed by @sumo-slonik here, reopened a new PR because of diff base change :D |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@shubham1206agra Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
|
||
| type WorkspaceRoomCreatePageProps = PlatformStackScreenProps<SettingsNavigatorParamList, typeof SCREENS.WORKSPACE.ROOM_CREATE>; | ||
|
|
||
| function WorkspaceRoomCreatePage({route}: WorkspaceRoomCreatePageProps) { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
This new component heavily duplicates the existing src/pages/workspace/WorkspaceNewRoomPage.tsx. The validation logic (lines 55-72), submit handler (lines 74-93), visibility/write-capability options construction (lines 95-109), and form JSX structure are nearly identical between the two files.
Consider extracting the shared room-creation logic (validation, submit, options building, form fields) into a shared hook or shared component that both WorkspaceNewRoomPage and WorkspaceRoomCreatePage can reuse. For example, a useRoomCreationForm hook could encapsulate the validation, submit, and options logic, while the two pages would only differ in their screen wrappers, headers, and navigation behavior.
Reviewed at: ae406ad | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
both serve different UX flow, so imo no need for logic extraction
There was a problem hiding this comment.
@jakubstec rather than maintain these two files, could we update WorkspaceNewRoomPage to accept an optional policyID route param?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae406ad2c9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… tests for that selectors
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
|
@shubham1206agra Hi, i've addressed the comment, should be ready now :) |
|
|
||
| type WorkspaceRoomCreatePageProps = PlatformStackScreenProps<SettingsNavigatorParamList, typeof SCREENS.WORKSPACE.ROOM_CREATE>; | ||
|
|
||
| function WorkspaceRoomCreatePage({route}: WorkspaceRoomCreatePageProps) { |
There was a problem hiding this comment.
@jakubstec rather than maintain these two files, could we update WorkspaceNewRoomPage to accept an optional policyID route param?
| <HeaderWithBackButton | ||
| title={translate('newRoomPage.createRoom')} | ||
| onBackButtonPress={() => Navigation.goBack(ROUTES.WORKSPACE_ROOMS.getRoute(policyID))} |
There was a problem hiding this comment.
for this goback, how are we planning to handle the Highlight-on-return animation for the newly created room in the list?
There was a problem hiding this comment.
@grgia I've implemented it in separate PR (draft): #91284 as it's blocked by Rooms List PR, and I didn't want to inflate LOC in a diff :) We can either merge them both (create button + room list), or merge room list first, then i'll just merge (create button + handle animation) to speed up the process
|
@shubham1206agra Do we have anything that needs fixing here, or is it already good as it is? |
Screen.Recording.2026-06-02.at.5.32.22.PM.mov@sumo-slonik The navigation after creating room does not happen |
Not navigating into the newly created room is the expected behavior here — it's stated explicitly in the parent issue requirements (#85043, Predesign decisions → Interactions):
So after creating a room we should drop the user back in the Rooms table, not navigate into the room chat. There is, however, a separate bug: the newly created room is not highlighted in the list on return. I'm already on it. The test steps in the PR description that say "you are navigated to room chat screen" are outdated, but I can't edit them since I'm not the author of this PR |
|
@sumo-slonik are you taking over the highlight in this PR? |
…-rooms/add-create-button-interaction
4729cc8 to
c0f5a30
Compare
|
@grgia It seems to me that it would be better to implement this highlight within this PR so that the adding feature is consistent and works correctly from the start. The highlight isn’t such a big change compared to the add itself. I’ve already added it to this PR. @shubham1206agra It’s ready for review. |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-06-02.at.9.36.03.PM.movScreen.Recording.2026-06-02.at.9.49.47.PM.mov |
|
@grgia I think we're ready to merge here |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.99-0 🚀
Bundle Size Analysis (Sentry): |
Help site review — no changes requiredThis PR adds an in-workspace Create button on the new Workspace → Rooms page, opening a room-creation screen locked to that workspace. After reviewing the diff against Why: The entire feature is gated behind the
The existing, generally-available way to create a room — global + → Start Chat → #Room tab — is unchanged and remains accurately documented in Create a New Chat ( Recommendation: Hold help site documentation until this feature graduates from the @jakubstec, please confirm this assessment. If you'd like the Workspace → Rooms page (including this Create flow) documented now despite the beta gating, reply with |
|
This PR failing because of the issue #92638 |
|
Deploy Blocker #92638 was identified to be related to this PR. |
|
Deploy Blocker #92645 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.99-9 🚀
|
Explanation of Change
Adds a Workspace Rooms Create Room integration, allowing to create new rooms within a workspace. Available under the route
workspaces/:policyID/rooms/new.Fixed Issues
$ #85043
$ #89725
Tests
Offline tests
Same as Tests.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
MacOS: Chrome / Safari
rooms1.mp4